Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594) #77506

Merged
merged 1 commit into from
Nov 10, 2022
Merged

[release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594) #77506

merged 1 commit into from
Nov 10, 2022

Conversation

ayakael
Copy link
Contributor

@ayakael ayakael commented Oct 26, 2022 •
edited
Loading

Backport of #66594

Per @omajid:

When packaging .NET on s390x through source-build, we want to keep debug
symbols enabled in the build. The package manager (debbuild, rpmbuild,
etc) generally strips the binaries to create distro-standard
-debug/-debuginfo packages. This was supported in coreclr via
--keepnativesymbols flag, but wasn't supported in mono (ie, s390x). Fix
that.

Customer impact

Source-build with mono-flavored runtime is problematic without this patch.

Testing

  • unit tests in ci
  • source-build build within Alpine environment passes

Risk

Low, as it is already in main, and only activates when KeepNativeSymbols!=true

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 26, 2022
@ayakael ayakael changed the title Fix KeepNativeSymbols to work on mono as well (backport of #66594) [release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594) Oct 26, 2022
@ayakael ayakael mentioned this pull request Oct 26, 2022
Closed
18 tasks
@teo-tsirpanis teo-tsirpanis added this to the 6.0.x milestone Oct 26, 2022
@build-analysis build-analysis bot mentioned this pull request Oct 26, 2022
Closed
@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from fab82ba to bc3084a Compare November 2, 2022 15:42
@carlossanlop
Copy link
Member

@marek-safar @directhex this needs to go through Tactics. When/if this is ready, please add the servicing-consider label and send an email to Tactics requesting merge approval.

@marek-safar marek-safar added the Servicing-consider Issue for next servicing release review label Nov 3, 2022
@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.12 Nov 3, 2022
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 3, 2022
@carlossanlop
Copy link
Member

@omajid @ayakael @directhex I see some CI failures that I haven't seen in any other backport PR. Are they related to this change?

@directhex can you please provide a code review sign-off?

@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from bc3084a to 3cec0ed Compare November 3, 2022 21:00
@build-analysis build-analysis bot mentioned this pull request Nov 4, 2022
Closed
2 tasks
@ayakael
Copy link
Contributor Author

ayakael commented Nov 4, 2022 •
edited
Loading

@omajid @ayakael @directhex I see some CI failures that I haven't seen in any other backport PR. Are they related to this change?

@directhex can you please provide a code review sign-off?

It really shouldn't be, as KeepNativeSymbols is really only used by source-build from memory, and the failures seem related to CoreCLR which this PR doesn't touch. Pushing a new rebase as to trigger another round of CI tests. Maybe it was just a fluke.

@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from 3cec0ed to 3b8eadb Compare November 4, 2022 20:21
@carlossanlop
Copy link
Member

Ping @directhex @marek-safar for a code review-sign off.

@carlossanlop carlossanlop mentioned this pull request Nov 10, 2022
Closed
@carlossanlop
Copy link
Member

CI failure confirmed offline to be unrelated. Signed off by area owners. Approved by Tactics. No OOB package authoring changes needed. Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 86caa85 into dotnet:release/6.0 Nov 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marek-safar marek-safar marek-safar approved these changes

@directhex directhex directhex approved these changes

Assignees

@marek-safar marek-safar

@directhex directhex

Labels
area-Build-mono community-contribution Indicates that the PR has been added by a community member Servicing-approved Approved for servicing release
Projects
None yet
Milestone
6.0.12
Development

Successfully merging this pull request may close these issues.
6 participants
@ayakael @carlossanlop @marek-safar @directhex @teo-tsirpanis @rbhanda